keyboard: Disconnect from input source manager when destroying indicator

When an InputSourceIndicator is destroyed, the InputSourceManager it was
connected to could (and probably will) outlive it (since the manager is
a singleton). If the InputSourceManager emits any subsequent signals,
the callbacks from the finalised InputSourceIndicator could be invoked,
and will reference finalised objects.

This can be triggered by running `pkexec true` from a gnome-terminal
window, then calling `pkill pkexec` from another terminal (on a
different VT or via SSH). This causes the dialogue to be cancelled by
polkitd.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/357

diff --git a/js/ui/status/keyboard.js b/js/ui/status/keyboard.js
index 980019cd4043d9b59dce3f08cd1c0a1fb81c098d..ef2a6049b28e4c6bd6a86c0490eced480fad0a58 100644 (file)
--- a/js/ui/status/keyboard.js
+++ b/js/ui/status/keyboard.js
@@ -832,6 +832,8 @@ var InputSourceIndicator = new Lang.Class({
     _init() {
         this.parent(0.0, _("Keyboard"));
 
+        this.connect('destroy', this._onDestroy.bind(this));
+
         this._menuItems = {};
         this._indicatorLabels = {};
 
@@ -856,11 +858,21 @@ var InputSourceIndicator = new Lang.Class({
         this._sessionUpdated();
 
         this._inputSourceManager = getInputSourceManager();
-        this._inputSourceManager.connect('sources-changed', this._sourcesChanged.bind(this));
-        this._inputSourceManager.connect('current-source-changed', this._currentSourceChanged.bind(this));
+        this._inputSourceManagerSourcesChangedId =
+            this._inputSourceManager.connect('sources-changed', this._sourcesChanged.bind(this));
+        this._inputSourceManagerCurrentSourceChangedId =
+            this._inputSourceManager.connect('current-source-changed', this._currentSourceChanged.bind(this));
         this._inputSourceManager.reload();
     },
 
+    _onDestroy() {
+        if (this._inputSourceManager) {
+            this._inputSourceManager.disconnect(this._inputSourceManagerSourcesChangedId);
+            this._inputSourceManager.disconnect(this._inputSourceManagerCurrentSourceChangedId);
+            this._inputSourceManager = null;
+        }
+    },
+
     _sessionUpdated() {
         // re-using "allowSettings" for the keyboard layout is a bit shady,
         // but at least for now it is used as "allow popping up windows